-
Notifications
You must be signed in to change notification settings - Fork 5.8k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Makefile: Replace Gometalinter to Golangci-lint #13405
Conversation
Codecov Report
@@ Coverage Diff @@
## master #13405 +/- ##
===========================================
Coverage 80.1718% 80.1718%
===========================================
Files 481 481
Lines 120586 120586
===========================================
Hits 96676 96676
Misses 16188 16188
Partials 7722 7722 |
--enable misspell \ | ||
--enable ineffassign \ | ||
check-static: tools/bin/golangci-lint | ||
tools/bin/golangci-lint run -v --disable-all --deadline=3m \ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why changed to 3m ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Indeed, golangci-lint is faster than gometalinter.
But it cannot support like --deadline 120s
, therefore I changed it to --deadline=3m
.
The reason for 3m rather than 2m , beacuse I think we should give more time for the new tool.
And changed it back when it stable enough.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
@@ -271,6 +268,8 @@ tools/bin/misspell:tools/check/go.mod | |||
tools/bin/ineffassign:tools/check/go.mod | |||
cd tools/check; \ | |||
$(GO) build -o ../bin/ineffassign github.com/gordonklaus/ineffassign | |||
tools/bin/golangci-lint: | |||
curl -sfL https://raw.githubusercontent.com/golangci/golangci-lint/master/install.sh| sh -s -- -b ./tools/bin v1.21.0 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Will the binary from this URL change? Is there any version control for tools?
If the make
result depends on the tool's version, it may trouble us sometimes. @xiekeyi98
We use go.mod to control the version of other tools here:
https://github.com/pingcap/tidb/blob/master/tools/check/go.mod
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Well, version control for tool is at last ( v1.21.0
).
What do you think about we should do to install it.? I use this tool: https://github.com/golangci/golangci-lint
LGTM |
Please resolve conflicts @xiekeyi98 |
/run-all-tests |
/run-all-tests |
What problem does this PR solve?
Because of Gometalinter DEPRECATED, we should change it to golangci-lint.
This change is
This PR will close #13675